Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(--format=json): Do not print header if specified format is json #1631

Merged
merged 2 commits into from
Jan 12, 2024

Conversation

aswasif007
Copy link
Contributor

@aswasif007 aswasif007 commented Jan 3, 2024

Description

Presently, if output format specified is json (--format=json), it prints a json object, but the output is prefixed by a key-value pair as header, which is not json. As a result, if someone wants to programmatically parse the json output, it would fail. In this PR, we are removing the header for json formats.

⚠️ The downside is, it also means we are dropping the data that falls outside json format. Let's discuss if that is accepable.

Before:

$ vip app 3202 --format=json
===================================
+ id: 3202
+ name: pmc-thr
+ repo: wpcomvip/pmc-thr
===================================
[
	{
		"id": 3202,
		"appId": 3202,
		"name": "production",
		"type": "production",
		"branch": "master",
		"currentCommit": "944a330",
		"primaryDomain": "www.hollywoodreporter.com",
		"launched": true
	},
...

After:

$ vip app 3202 --format=json
[
	{
		"id": 3202,
		"appId": 3202,
		"name": "production",
		"type": "production",
		"branch": "master",
		"currentCommit": "944a330",
		"primaryDomain": "www.hollywoodreporter.com",
		"launched": true
	},
...

How to test

  1. Pull the PR
  2. rm -rf dist && npm run build && npm link
  3. Run node ./dist/bin/vip app 3202 and node ./dist/bin/vip app 3202 --format=json

@aswasif007 aswasif007 requested a review from a team January 3, 2024 11:33
src/lib/cli/command.js Outdated Show resolved Hide resolved
@saroshaga
Copy link
Contributor

The downside is, it also means we are dropping the data that falls outside json format. Let’s discuss if that is accepable.

can you post a before and after object structure?

@aswasif007 aswasif007 force-pushed the bb8-10508/fix/jsonformat branch from 054cd85 to d3db240 Compare January 12, 2024 04:27
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@aswasif007 aswasif007 merged commit 5a6507d into trunk Jan 12, 2024
14 checks passed
@aswasif007 aswasif007 deleted the bb8-10508/fix/jsonformat branch January 12, 2024 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants